-
Notifications
You must be signed in to change notification settings - Fork 750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Driver][SYCL] warning emitted when compiling wrapped object #16397
Conversation
After the switch to using clang to build the wrapped object the embedded target in the IR is caused a warning to be emitted. Suppress this warning by passing -Wno-override-module. Warning emitted: warning: overriding the module target triple with x86_64-pc-windows-msvc19.29.30154 [-Woverride-module]
@@ -10344,9 +10344,16 @@ void OffloadWrapper::ConstructJob(Compilation &C, const JobAction &JA, | |||
|
|||
if (WrapperCompileEnabled) { | |||
// TODO Use TC.SelectTool(). | |||
// Pass -Wno-override-module to the compilation to restrict the warning | |||
// that is emitted due to the target in the generated IR from the wrapping | |||
// step. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
due to the target in the generated IR from the wrapping step.
Can you elaborate this sentence?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The clang-offload-wrapper
will wrap the device object in an IR file that needs to be compiled. The -host
setting sets up the target triple. I did some additional experimentation and I believe we may be able to fix this by providing a more complete -host
value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The -host setting sets up the target triple. I did some additional experimentation and I believe we may be able to fix this by providing a more complete -host value.
Can you clarify which -host
setting is being referred here?
Also, the example warning mentioned in the description (x86_64-pc-windows-msvc) specifies a windows target triple.
Will this change based on the --target
value?
Can a test be added to verify if the warning is not thrown when -Wno-override-module
is passed?
Lastly, why do we need to suppress this warning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm referring to the -host
option that is used during the clang-offload-wrapper
call. This information is used when generating the IR for the wrapped binary. Having the -host
value for the clang-offload-wrapper
call match the --target
value for the clang
call should prevent the diagnostic from being emitted.
The changes I am working on shouldn't require the -Wno-override-module
option anymore.
As for a test, a driver specific test probably isn't ideal here, as the warning is only emitted during the device linking stage. What is interesting here, is that the diagnostic is emitted from an internal call to 'clang' and there is no way to disable it.
TCArgs.MakeArgString("--target=" + TC.getAuxTriple()->str()), "-c", | ||
"-o", Output.getFilename(), WrapperFileName}; | ||
TCArgs.MakeArgString("--target=" + TC.getAuxTriple()->str()), | ||
"-Wno-override-module", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can a test be added to verify that the warning no longer occurs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will require a non-driver LIT test to verify (some kind of E2E test that allows for full compilation). OK to add later?
Intead of using -Wno-override-module during the clang compile, adjust how we call clang-offload-wrapper so the used triple matches expectations during the clang call.
TCArgs.MakeArgString("--target=" + TC.getAuxTriple()->str()), "-c", | ||
"-o", Output.getFilename(), WrapperFileName}; | ||
ArgStringList ClangArgs{TCArgs.MakeArgString("--target=" + HostTripleStr), | ||
"-c", "-o", Output.getFilename(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There appears to be extra whitespaces added here and in L10353
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a formatting change suggested by clang-format
when the HostTripleStr
was updated.
@intel/llvm-gatekeepers , this looks ready for merge - thanks! |
After the switch to using clang to build the wrapped object the embedded target in the IR is caused a warning to be emitted. Fix this by adjusting the clang-offload-wrapper call to use the same host triple that is used when performing a regular compilation. Matching these up allows for no diagnostic to be emitted.
Warning emitted: warning: overriding the module target triple with
x86_64-pc-windows-msvc19.29.30154 [-Woverride-module]